Haie / L'historique des dossiers inclut la mise en attente + champs de l'état du dossier (Attente PC 3/3) + analytics #918

pyDez · 2025-12-11T09:12:19Z

https://trello.com/c/EQ4qj2or/1963-haie-lhistorique-des-dossiers-inclut-la-mise-en-attente-champs-de-l%C3%A9tat-du-dossier-attente-pc-3-3-analytics

tristanrobert · 2025-12-11T09:12:37Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

numahell

J'ai des commentaires pour plus de commentaires :)
à part ça c'est bon pour moi.

envergo/static/sass/project_haie.scss

envergo/petitions/views.py

numahell

Merci pour l'ajout des commentaires et des docstrings :)

thibault

J'ai posé quelques commentaires, j'ai l'intuition que la solution choisie risque de créer des problèmes quand on prendra en compte plus d'éléments dans l'historique de modification du dossier. Qu'en penses-tu ?

envergo/petitions/models.py

thibault · 2026-01-05T10:47:40Z

envergo/petitions/views.py


    def get_context_data(self, **kwargs):
+
+        def extract_entries(log):


Je suis un peu mitigé quant à ce choix technique. À ce stade, on a une belle correspondance ou chaque entrée StatusLog correspond à un changement d'état. C'est clair, explicite et propre.

Est-ce que ça n'aurait pas été plus simple de garder cette correspondance en créant tout simplement une nouvelle entrée au moment de la reprise de l'instruction ? Ça aurait évité de bidouiller pour créer ce tableau et épargné la nécessité de créer une nouvelle structure de données.

Par ailleurs, la présente solution créé un petit bug de cohérence sur le nombre de changements indiqué.

Pour ce besoin précis d'affichage des changements, on prefererait effectivement avoir un objet distinct pour chaque changement: changement d'état, demande de suppression, reception des compléments.
Mais il y a peut etre eu d'autre raison technique ou metier qui ont mené à la decision de faire porter les demandes de supplement et les reception directement sur les changements d'état.
Je ne suis pas fermé à lancer la discussion, mais peut etre en dehors de ce ticket que je preferais merger tel quel.

Je reste super mitigé vis à vis de ça, parce qu'on intègre au dépôt du code relativement complexe pour quelque chose qui devrait être trivial : une liste d'états = un tableau d'historique. Pour moi ça frise pas mal la dette technique parce que dès qu'on devra toucher à ce changement d'état, il faudra retoucher cette boucle. Je trouve dommage d'intégrer ça au dépôt si ça doit être retouché plus tard.

D'ailleurs, il me semble qu'il y a déjà un ticket dans les tuyaux qui va obliger à se poser la question.

https://trello.com/c/ecg5IsPW/2057-conserver-l%C3%A9tat-suspendu-lors-dun-changement-d%C3%A9tape

Maintenant, si tu juges que c'est mieux de fusionner en état pour des raisons de mep et que tu veux y revenir plus tard, je ne bloque pas la fusion :)

J'ai suivi la voie de la sagesse (aka @thibault) voici la séparation des logs en différents objets ! Une nouvelle revue n'est pas de refus :)

envergo/petitions/views.py

thibault · 2026-01-05T10:59:05Z

envergo/templates/haie/petitions/instructor_view_procedure.html

-                      {% if log.previous_log %}
-                        {{ log.previous_log.get_stage_display }}
-                        {% if log.previous_log.decision != "unset" %}
+                      {% if log.type == "status_change" %}


Ça me semble beaucoup de logique pour de l'affichage. Est-ce que ça n'aurait pas plutôt sa place dans le modèle, avec un champ « type » de type Choices ?

```{{ log.get_type_display }}``

envergo/templates/haie/petitions/instructor_view_procedure.html

Add data migration step to set resumed_by for existing StatusLog entries that have info_receipt_date but were missing resumed_by. Use atomic=False to avoid pending trigger events error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

thibault · 2026-01-12T09:22:29Z

envergo/petitions/views.py


    def get_context_data(self, **kwargs):
+
+        def extract_entries(log):


Je reste super mitigé vis à vis de ça, parce qu'on intègre au dépôt du code relativement complexe pour quelque chose qui devrait être trivial : une liste d'états = un tableau d'historique. Pour moi ça frise pas mal la dette technique parce que dès qu'on devra toucher à ce changement d'état, il faudra retoucher cette boucle. Je trouve dommage d'intégrer ça au dépôt si ça doit être retouché plus tard.

D'ailleurs, il me semble qu'il y a déjà un ticket dans les tuyaux qui va obliger à se poser la question.

https://trello.com/c/ecg5IsPW/2057-conserver-l%C3%A9tat-suspendu-lors-dun-changement-d%C3%A9tape

Maintenant, si tu juges que c'est mieux de fusionner en état pour des raisons de mep et que tu veux y revenir plus tard, je ne bloque pas la fusion :)

thibault

Merci beaucoup d'avoir cédé à mes réclamations de refactoring.

J'espère que tu ne vas pas croire que je m'acharne mais de mon point de vue il reste des éléments problématiques sur cette PR.

D'abord, le fait de récupérer l'état courant d'un dossier nécessite de trouver la bonne entrée de l'historique qui détient l'information, ce qui fait exploser le nombre de requête sur la liste des dossiers.

Ensuite, ça amène une complexité assez importante sur certaines méthodes qui devraient être de simples accesseurs.

Est-ce que tu as considéré la possibilité que chaque objet StatusLog soit un instantané du dossier avec toutes les variables utiles ? Il me semble que ça rendrai trivial toutes les requêtes :

récupérer l'état en pause / repris du dossier = récupérer le dernier log
récupérer la due date = récupérér la due date du dernier log
construire le tableau des changements = simplement faire un diff entre deux états.

project = self.object
logs = project.status_history.all()
changes = [diff(pair[0], pair[1]) for pair in pairwise(logs)]

Ou il y a des éléments auxquels je ne pense pas qui font que c'est un mauvais plan ?

Cf. peut-être le pattern memento : https://refactoring.guru/fr/design-patterns/memento

thibault · 2026-01-19T15:23:31Z

envergo/petitions/models.py

-        log = self.status_history.first()
-        return log
+        """Get the latest status_change log."""
+        return self.status_history.filter(type=LOG_TYPES.status_change).first()


Petit problème de perfs avec ces requêtes, notamment sur la page de liste des dossiers.

thibault · 2026-01-19T15:30:45Z

envergo/petitions/models.py

    @property
    def due_date(self):
-        return self.current_status.due_date if self.current_status else None
+        if self.is_paused:


Ça me semble un code smell que cette méthode soit si complexe. Je viens de la relire 10 fois et je ne parviens toujours pas à la comprendre.

J'aurais tendance à penser qu'il y a peut-être une question d'architecture à corriger si la solution actulle amène à du code aussi complexe ?

Par exemple, si l'objet « StatusLog » est un instantané de l'état du dossier, pourquoi est-ce que cela ne suffit pas de récupérer le dernier état pour vérifier sa valeur « due_date » ?

thibault · 2026-01-19T15:32:01Z

envergo/petitions/models.py

+        resumption = self.latest_resumption
+        if not resumption:
+            return True
+        return suspension.created_at > resumption.created_at


Même commentaire que plus haut. Pourquoi ne pas avoir un simple return latest_status.is_paused ?

thibault · 2026-01-19T15:41:33Z

envergo/petitions/models.py

-    """A petition project status (stage + decision) change log entry."""
+    """A petition project status log entry.
+
+    Each entry represents one of:


J'ai l'impression qu'on est sur une assez classique opposition entre de l'impératif et du déclaratif.

Là, si je comprends bien, on définit un statut comme « voici la modif par rapport au statut précédent ». Ce qui oblige, pour connaître l'état du projet, à reconstruire là donnée à partir d'une liste de statuts, et ce qui fait dépendre la validité d'un statut des statuts précédents.

Par ailleurs, la distinction entre « status_change » et « suspension / resumption » me semble artificielle. Si on a un dossier suspendu, pourquoi ne pourrait-on pas reprendre l'instruction ET assigner un nouveau statut en même temps ?

Ne serait-ce pas plus clair de considérer un statut comme « voici un instantané du dossier » ? Ainsi, connaître l'état du dossier ne nécessiterait que de récupérer un seul objet en base ?

pyDez added 3 commits December 11, 2025 09:38

rework procedure table layout

dfd19a1

analytics

2675ec9

None author

4aac306

pyDez requested a review from thibault December 11, 2025 09:12

pyDez requested a review from numahell December 11, 2025 09:12

pyDez added 2 commits December 15, 2025 08:45

move some info

a3a712d

author for suspension

a70ed24

numahell reviewed Dec 15, 2025

View reviewed changes

pyDez added 2 commits December 18, 2025 06:27

Merge remote-tracking branch 'origin/main' into history-layout

040cf9c

comment + resumption author

b51491d

pyDez requested a review from numahell January 5, 2026 08:13

numahell approved these changes Jan 5, 2026

View reviewed changes

thibault reviewed Jan 5, 2026

View reviewed changes

pyDez and others added 6 commits January 7, 2026 06:32

Merge remote-tracking branch 'origin/main' into history-layout

08ed666

add DB constraint and use django

95ef96b

clean template

67becdc

linter

668067f

use the first user

a5503c7

pyDez requested a review from thibault January 7, 2026 10:03

pyDez added 2 commits January 9, 2026 11:24

Merge remote-tracking branch 'origin/main' into history-layout

f5c951c

fix post

409ee80

thibault approved these changes Jan 12, 2026

View reviewed changes

pyDez added 3 commits January 14, 2026 12:02

split logs: status change, suspension and resumption as distinct objects

b9d7eeb

fix tests

6acb287

fix migration

63386b4

pyDez requested a review from thibault January 15, 2026 08:48

pyDez added 2 commits January 19, 2026 10:27

Merge remote-tracking branch 'origin/main' into history-layout

c396e8d

replace response_due_date by due_date + cosmetic

5e02c97

disable project status change while paused

669d1f3

thibault reviewed Jan 19, 2026

View reviewed changes


		def get_context_data(self, **kwargs):

		def extract_entries(log):

Haie / L'historique des dossiers inclut la mise en attente + champs de l'état du dossier (Attente PC 3/3) + analytics #918

Are you sure you want to change the base?

Haie / L'historique des dossiers inclut la mise en attente + champs de l'état du dossier (Attente PC 3/3) + analytics #918

Uh oh!

Conversation

pyDez commented Dec 11, 2025

Uh oh!

tristanrobert commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

numahell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

numahell left a comment

Choose a reason for hiding this comment

Uh oh!

thibault left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thibault Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thibault left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tristanrobert commented Dec 11, 2025 •

edited

Loading

thibault Jan 5, 2026 •

edited

Loading